Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datahub: support thesaurus based advanced fields #719

Merged
merged 10 commits into from
Dec 11, 2023

Conversation

fgravin
Copy link
Member

@fgravin fgravin commented Dec 9, 2023

Depends on #718

  • Fix translations
  • Remove the dependency from Fields to Gn4ToolApi (for translation package)
  • Add thesaurus based Field
  • Add inspire theme field
  • Add a resource contact field

The contact field will be handled in another PR, as I figured out that it is the same as the OrganizationField with the metadata strategy.

as we search on a key term, not on a simple term
fields is in the domain and should not depend on gn4 repository implementation.
this should pass through the platform interface abstraction
to fetch all keywords in the requested language.
Implementation done for gn4, note that it uses a search request, with a limit of 1000, will be an issue in the long run
@fgravin fgravin added bug Something isn't working enhancement Proposal for a new feature labels Dec 9, 2023
@fgravin fgravin added this to the 2.1.0 milestone Dec 9, 2023
@fgravin fgravin self-assigned this Dec 9, 2023
Copy link
Contributor

github-actions bot commented Dec 9, 2023

Affected libs: api-repository, feature-catalog, feature-record, feature-router, feature-search, feature-map, feature-dataviz, feature-auth, common-domain, api-metadata-converter, feature-editor, ui-search, common-fixtures, ui-elements, ui-catalog, util-shared, ui-widgets, ui-inputs, ui-map, ui-dataviz, data-access-gn4,
Affected apps: datahub, metadata-editor, demo, webcomponents, search, map-viewer, datafeeder, metadata-converter, data-platform,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

@fgravin fgravin mentioned this pull request Dec 9, 2023
5 tasks
as /sourceS/harvester is not compatible in 4.2.2
Copy link
Collaborator

@Angi-Kinas Angi-Kinas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your help with this task, your solution looks a lot cleaner.
The PR looks good to me 👍

@@ -4,7 +4,7 @@
class="py-[37px] pl-[47px] rounded-lg border bg-white mb-5 card-shadow cursor-pointer"
[figure]="recordsCount$ | async"
[icon]="'folder_open'"
[title]="'catalog.figures.datasets'"
title="catalog.figures.datasets"
Copy link
Collaborator

@jahow jahow Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use the translate pipe here, instead of marker? I know it's translated too in the gn-ui-figure component but amaybe that's the thing that doesn't make sense (translating inputs in presentation components mean the translation keys will not be discoverable)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I just saw the approval from Angie.
I've merged it.

I also found it weird that the translation is done in the children, I understood that it's because in the children component, the translations use params.

    <div translate class="title truncate" [translateParams]="{ count: figure }">
      {{ title }}
    </div>

But yes, translations should be done from the parent IMO.

Comment on lines +104 to +106
translateKey(key: string): Observable<string> {
return this.keyTranslations$.pipe(map((translations) => translations[key]))
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fgravin fgravin merged commit 2f085bb into gn4-api-update Dec 11, 2023
7 of 8 checks passed
@fgravin fgravin deleted the rework-filters branch December 11, 2023 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement Proposal for a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants